-
Notifications
You must be signed in to change notification settings - Fork 7
Breez SDK token support #199
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
45912a0 to
158783f
Compare
dangeross
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think we'll hit issues with the u128 casting? Like with the max supply
Definitely! The idea is to address that in a separate PR. |
0129e56 to
7d223d9
Compare
41a942c to
fc35449
Compare
|
PR is ready for review - still keeping as a draft just because it's missing the query ordering field. As a result, payments are currently not properly synced, but they can still be received or sent. |
1865a79 to
ba0a2df
Compare
ba0a2df to
f5749cc
Compare
roeierez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
9ed94d5 to
e87a192
Compare
|
I've added a commit that introduces the use of sparkscan to sync payments. Unfortunately, for now, there's no good way for us to use SparkScan to query pending ones (no batch option to query transactions by id, and querying 1 by 1 will likely lead to reaching api limits), so to do that, we still use the SO api. We may switch to a full sparkscan-based approach when batching is available. Other noteworthy changes:
I still need to add tests and test more, but I would appreciate an initial pass of the current state of the PR. |
roeierez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I very like that. I like the fact we sync pending payments for now separately, seems less vulnerable to sync issues.
roeierez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
e87a192 to
c5d08cb
Compare
c5d08cb to
c36def8
Compare
9182f6b to
9c70b27
Compare
dangeross
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
roeierez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Added my final comments.
roeierez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
This PR adds support for token Payments on the Breez SDK API.
Main todos:
Out of scope (left for separate PRs):
receive_paymentin a follow-up PR: Spark invoice support #212